Skip to content

MPT-14896 E2E for catalog price list items#142

Merged
albertsola merged 1 commit intomainfrom
e2e/catalog/MPT-14896-price-lists-items
Dec 2, 2025
Merged

MPT-14896 E2E for catalog price list items#142
albertsola merged 1 commit intomainfrom
e2e/catalog/MPT-14896-price-lists-items

Conversation

@albertsola
Copy link
Contributor

@albertsola albertsola commented Dec 2, 2025 •
edited by coderabbitai bot
Loading

Summary by CodeRabbit

  • Refactor

    • Streamlined price list items service to support GET and UPDATE operations.

  • Tests

    • Added comprehensive end-to-end and unit test coverage for price list items retrieval, filtering, updates, and error handling scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 2, 2025 •
edited
Loading

Walkthrough

The PR refactors PriceListItemsService and AsyncPriceListItemsService to replace ManagedResourceMixin with granular GetMixin and UpdateMixin, reflecting that these services only support read and update operations. Comprehensive end-to-end tests are added for both sync and async price list item operations, along with supporting fixtures and updated unit tests.

Changes

Cohort / File(s) Summary
Service Refactoring
mpt_api_client/resources/catalog/price_list_items.py		 Replaced ManagedResourceMixin with GetMixin and UpdateMixin for both sync and async service classes; updated imports accordingly to reflect granular capability inheritance.
E2E Test Fixtures
tests/e2e/catalog/price_lists/items/conftest.py		 Added pytest fixtures: price_list_items_service, async_price_list_items_service, price_list_item_data, and price_list_item for end-to-end test support.
E2E Test Suite
tests/e2e/catalog/price_lists/items/test_sync_price_list_items.py, test_async_price_list_items.py		 Added comprehensive end-to-end tests covering retrieval, filtering with pagination, updates, 404 error handling, and validation of invalid update data for both sync and async operations.
Unit Test Updates
tests/unit/resources/catalog/test_price_list_items.py		 Removed test parametrization for "create" and "delete" methods; retained only "get" and "update" method coverage to align with supported capabilities.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Key areas requiring attention:
    • Verify mixin replacement correctly exposes only get and update methods with no unintended side effects
    • Confirm test fixtures are properly scoped and reusable across test modules
    • Review e2e test coverage for price list items to ensure comprehensive API interaction validation
    • Validate that unit test changes accurately reflect the reduced method set

Suggested reviewers

  • d3rky
  • jentyk
  • alephsur
  • robcsegal
  • ruben-sebrango

Poem

🐰 The mixins split with purpose bright,
GetMixin, UpdateMixin take flight,
Tests unfold in sync and async grace,
E2E checks keep pace,
Price lists now confined just right! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly references the main change: adding end-to-end tests for catalog price list items as indicated by the PR number and all file changes.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch e2e/catalog/MPT-14896-price-lists-items

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Dec 2, 2025 •
edited by atlassian bot
Loading

✅ Found Jira issue key in the title: MPT-14896

Generated by 🚫 dangerJS against c17a870

@albertsola albertsola force-pushed the e2e/catalog/MPT-14896-price-lists-items branch from c035f73 to c17a870 Compare December 2, 2025 16:42
@albertsola albertsola marked this pull request as ready for review December 2, 2025 16:43
coderabbitai[bot]
coderabbitai bot reviewed Dec 2, 2025
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
tests/e2e/catalog/price_lists/items/test_async_price_list_items.py (1)

38-42: Consider renaming test_create_price_list_item_invalid_data for clarity

This test calls update on an existing item rather than performing a create. Renaming to something like test_update_price_list_item_invalid_data would better reflect the current service interface and avoid confusion now that create isn’t part of the API.

tests/e2e/catalog/price_lists/items/conftest.py (1)

4-26: Well-structured fixtures; consider guarding against empty pages

The fixtures nicely centralize construction of sync/async services and reusable item data for the E2E suite. One minor consideration: price_list_item assumes fetch_page(1) always returns at least one item. If there’s any chance of an empty list in certain environments, adding a simple assertion or fallback before indexing (price_list_items[0]) would make failures easier to diagnose.

tests/e2e/catalog/price_lists/items/test_sync_price_list_items.py (1)

36-40: Align invalid-data test name with actual behaviour

test_create_price_list_item_invalid_data exercises an update call on an existing item. Renaming the test to reference “update” instead of “create” would better reflect what’s happening and stay consistent with the service’s supported operations.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8f72e0c and c17a870.

📒 Files selected for processing (5)
  • mpt_api_client/resources/catalog/price_list_items.py (3 hunks)
  • tests/e2e/catalog/price_lists/items/conftest.py (1 hunks)
  • tests/e2e/catalog/price_lists/items/test_async_price_list_items.py (1 hunks)
  • tests/e2e/catalog/price_lists/items/test_sync_price_list_items.py (1 hunks)
  • tests/unit/resources/catalog/test_price_list_items.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
tests/e2e/catalog/price_lists/items/test_async_price_list_items.py (5)
mpt_api_client/exceptions.py (1)
  • MPTAPIError (20-42)
tests/e2e/helper.py (1)
  • assert_async_service_filter_with_iterate (31-39)
tests/e2e/catalog/price_lists/items/test_sync_price_list_items.py (5)
  • test_get_price_list_item (11-14)
  • test_filter_price_list_items (17-20)
  • test_update_price_list_item (23-26)
  • test_get_price_list_item_not_found (29-33)
  • test_create_price_list_item_invalid_data (36-40)
tests/e2e/catalog/price_lists/items/conftest.py (3)
  • async_price_list_items_service (10-11)
  • price_list_item (24-26)
  • price_list_item_data (15-20)
mpt_api_client/models/model.py (1)
  • id (119-121)
tests/e2e/catalog/price_lists/items/conftest.py (2)
tests/e2e/conftest.py (3)
  • mpt_ops (31-32)
  • async_mpt_ops (36-39)
  • short_uuid (101-102)
tests/e2e/catalog/price_lists/conftest.py (1)
  • price_list_id (29-30)
tests/e2e/catalog/price_lists/items/test_sync_price_list_items.py (4)
mpt_api_client/exceptions.py (1)
  • MPTAPIError (20-42)
tests/e2e/helper.py (1)
  • assert_service_filter_with_iterate (42-50)
tests/e2e/catalog/price_lists/items/conftest.py (3)
  • price_list_items_service (5-6)
  • price_list_item (24-26)
  • price_list_item_data (15-20)
tests/unit/resources/catalog/test_price_list_items.py (1)
  • price_list_items_service (10-13)
tests/unit/resources/catalog/test_price_list_items.py (17)
tests/unit/resources/catalog/test_pricing_policy_attachments.py (1)
  • test_methods_present (44-47)
tests/unit/resources/accounts/test_account_users.py (1)
  • test_methods_present (27-30)
tests/unit/resources/accounts/test_accounts_users.py (1)
  • test_methods_present (43-46)
tests/unit/resources/billing/test_credit_memo_attachments.py (1)
  • test_methods_present (42-45)
tests/unit/resources/billing/test_custom_ledger_charges.py (1)
  • test_methods_present (40-43)
tests/unit/resources/billing/test_custom_ledger_attachments.py (1)
  • test_methods_present (40-43)
tests/unit/resources/billing/test_invoice_attachments.py (1)
  • test_methods_present (41-44)
tests/unit/resources/billing/test_journal_attachments.py (1)
  • test_methods_present (41-44)
tests/unit/resources/billing/test_journal_sellers.py (1)
  • test_methods_present (38-41)
tests/unit/resources/billing/test_journal_upload.py (1)
  • test_methods_present (36-39)
tests/unit/resources/billing/test_journal_charges.py (1)
  • test_methods_present (38-41)
tests/unit/resources/billing/test_invoices.py (1)
  • test_methods_present (27-30)
tests/unit/resources/billing/test_ledger_charges.py (1)
  • test_methods_present (36-39)
tests/unit/resources/billing/test_ledger_attachments.py (1)
  • test_methods_present (41-44)
tests/unit/resources/billing/test_statement_charges.py (1)
  • test_methods_present (40-43)
tests/unit/resources/catalog/test_product_terms.py (1)
  • test_methods_present (42-45)
tests/e2e/catalog/price_lists/items/conftest.py (1)
  • price_list_items_service (5-6)
mpt_api_client/resources/catalog/price_list_items.py (1)
mpt_api_client/http/mixins.py (4)
  • AsyncGetMixin (380-395)
  • AsyncUpdateMixin (286-300)
  • GetMixin (361-377)
  • UpdateMixin (41-55)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (4)
mpt_api_client/resources/catalog/price_list_items.py (1)

2-8: Mixins now correctly reflect read/update-only capabilities

Switching PriceListItemsService and AsyncPriceListItemsService to use GetMixin/UpdateMixin (and their async counterparts) alongside the collection mixins cleanly expresses the supported operations and aligns with the new tests. MRO still looks correct for _resource_action and config attributes; no issues spotted.

Also applies to: 25-37

tests/e2e/catalog/price_lists/items/test_async_price_list_items.py (1)

1-42: Good async coverage for get/filter/update and error cases

Async tests exercise the main behaviours (successful get/filter/update plus 404 and 400 error paths) using the shared helper and fixtures, which matches the new service surface (get/update only). Structure and assertions look consistent with the sync suite.

tests/unit/resources/catalog/test_price_list_items.py (1)

39-39: Method presence checks now aligned with service capabilities

Restricting the parametrized methods to ["get", "update"] for both sync and async services correctly reflects the move away from managed (create/delete) operations and keeps the unit tests in sync with the new mixin-based design.

Also applies to: 46-46

tests/e2e/catalog/price_lists/items/test_sync_price_list_items.py (1)

1-40: Comprehensive sync E2E coverage matches the new service surface

These tests cover the key behaviours of the sync price list items service: happy-path get/filter/update plus 404 and 400 error paths, using the shared fixtures and helper. This lines up well with the refactor to get/update-only capabilities.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 2, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@albertsola albertsola self-assigned this Dec 2, 2025
@albertsola albertsola merged commit c24cde9 into main Dec 2, 2025
6 of 7 checks passed
@albertsola albertsola deleted the e2e/catalog/MPT-14896-price-lists-items branch December 2, 2025 17:25
This was referenced Dec 4, 2025
Merged
Merged
Merged
@coderabbitai coderabbitai bot mentioned this pull request Dec 12, 2025
Merged
@coderabbitai coderabbitai bot mentioned this pull request Dec 29, 2025
Merged
This was referenced Jan 7, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@ruben-sebrango ruben-sebrango ruben-sebrango approved these changes

@robcsegal robcsegal robcsegal approved these changes

@d3rky d3rky Awaiting requested review from d3rky

@jentyk jentyk Awaiting requested review from jentyk

@svazquezco svazquezco Awaiting requested review from svazquezco

@alephsur alephsur Awaiting requested review from alephsur

Assignees

@albertsola albertsola

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@albertsola @ruben-sebrango @robcsegal